Note the pattern if ( ... .isDone() ) { get() } should be avoided in general, because it's sensitive to TOCTOU. That's why I suggested var value = getNow(null); if ( value != null) .

Though in this case... I think the worst that could happen is a missed opportunity, so it's probably fine.

What I'm afraid of using getNow(null) in this case, it's that we will return null a bit too soon.

In this specific case, when .isDone() returns true, we should know for a fact that there's a result that we can return.

When it returns false, it's possible that by the time we call .getNow(null), the result is not null and we have created a ProxyConnection even if we didn't need to, but my guess is that this use case won't happen often.

Though in this case... I think the worst that could happen is a missed opportunity, so it's probably fine.

I didn't notice this line. Yes, that's what I was thinking.
The initial solution could have potentially caused a NullPointerException instead.

So the opened check here is supposed to handle a case where someone does something like this?

<trigger connection request>
connection = connection.withBatchSize(...); // Connection not yet initialized somehow?
connection.doSomething();

That seems odd... did you do this because you noticed it actually happening?

In any case, +1 to be on the safe side.

It's to be on the safe side.

Looking at the code, it seems to me there's a small window between setting opened to true and completing completableFuture with the connection. So, if opened it's true and completableFuture is not done, .getNow(null) will return null (and cause a NullPointerException), even if the connection is getting opened.

It's a bit late, I think I will sleep over it and merge it tomorrow morning if there are no other comments.
After that I will start the release.

Thanks